Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Modify AdvancedTreeSearch for CtcTreeBuilder support #102

Open
wants to merge 34 commits into
base: master
Choose a base branch
from
Open

Modify AdvancedTreeSearch for CtcTreeBuilder support #102

wants to merge 34 commits into from
+48 −14

Conversation

larissakl
Copy link
Contributor

Just minor adjustments to make the AdvancedTreeSearch work with the CtcTreeBuilder:

  • Modified two if-statements related to the stateDepths which caused segmentation faults due to the self-loops in the CTC tree.

  • Two assertions didn’t apply to the CTC tree, as its root state logic is significantly simpler. These assertions are now only checked if the CtcTreeBuilder is not used, so the previous behavior remains unchanged.

larissakl and others added 21 commits December 10, 2024 18:57
@larissakl
Refactored TreeBuilder
0cef183
Apply formatting
ed7430d
Fix "historyLenght" name
7cc2e2f
@larissakl
replace vector trees_ by one tree_ and refactor related code
8bda825
@larissakl
Merge branch 'master' into treebuilder_refactor-tree-vector
f550faa 
# Conflicts:
#	src/Search/AdvancedTreeSearch/PersistentStateTree.cc
#	src/Search/AdvancedTreeSearch/TreeBuilder.cc
Apply formatting
ac690d0
@larissakl
Update comments and tree_ initialization
b947fe6
@larissakl
Add CtcTreeBuilder
f1691a3
@larissakl
Uncouple treebuilding from AdvancedTreeSearch
eafef92
@curufinwe
Merge branch 'master' into ctc-treebuilder
fd30f7d
@larissakl
replace std::vector by std::set
dea490c
@larissakl
Merge branch 'ctc-treebuilder' into uncouple-treebuilder
b229128
@larissakl
Make label loops optional for RNA topology
64a6c50
Formatting
710505f
Merge branch 'master' into uncouple-treebuilder
55f8f0e
Merge branch 'uncouple-treebuilder' into treebuilder-rna-topology
5125c84
@larissakl
Rename "labelLoops" to "enableLabelLoop"
4a88b3c
@larissakl
Move createTreeBuilder to Search::Module and remove subdir
9a5d218
@larissakl
Merge branch 'uncouple-treebuilder' into treebuilder-rna-topology
5990bc6 
# Conflicts:
#	src/Search/TreeBuilder.hh
@larissakl
Introduce RnaTreeBuilder subclass and allow-label-loop config-parameter
d8149f8
@larissakl
Adjust AdvancedTreeSearch for CtcTreeBuilder support
2e9c51b
@larissakl larissakl requested review from curufinwe and SimBe195 and removed request for curufinwe March 1, 2025 16:34
@larissakl
Merge branch 'master' into treebuilder-rna-topology
552b5a1 
# Conflicts:
#	src/Search/AdvancedTreeSearch/AcousticLookAhead.hh
#	src/Search/AdvancedTreeSearch/LanguageModelLookahead.hh
#	src/Search/AdvancedTreeSearch/LinearPrediction.hh
#	src/Search/AdvancedTreeSearch/SearchSpace.cc
#	src/Search/AdvancedTreeSearch/SearchSpace.hh
#	src/Search/Module.cc
#	src/Search/Module.hh
#	src/Search/PersistentStateTree.cc
#	src/Search/TreeBuilder.cc
#	src/Search/TreeBuilder.hh
#	src/Search/TreeStructure.hh
@larissakl
Move allow-label-loop parameter to CtcTreeBuilder and RnaTreeBuilder
9deba08
@larissakl
Merge branch 'master' into advanced-tree-search-ctc
8670b7e
@larissakl
Adjust verify statements
b23b180
@larissakl
Merge branch 'treebuilder-rna-topology' into advanced-tree-search-ctc
2c8669a
@larissakl larissakl changed the base branch from master to treebuilder-rna-topology March 20, 2025 15:25
@larissakl
Copy link
Contributor Author

The AdvancedTreeSearch can be used with the CtcTreeBuilder and RnaTreeBuilder in two ways:

  1. Enable loops (default setting) and set the TDP for loop to infinity
  2. Disable loops (set allow-label-loop and allow-blank-loop to no) and set the TDP for loop to some value, e.g. 0.0

For both configurations, the TDP for skip has to be set to infinity.
Until now, the LM lookahead can only be used with option 2, with option 1 it will result in a segmentation fault.
Additionally, when label loops are disabled, the tree includes skip-transitions between identical labels, which are not allowed in the CTC-topology. This issue must be handled in the search code. I added a TODO in AdvancedTreeSearch for this.

Base automatically changed from treebuilder-rna-topology to master March 21, 2025 09:55
@larissakl
Copy link
Contributor Author

Maybe you have a better idea for naming the config parameter "force-blank-between-identical-labels"...

curufinwe
curufinwe requested changes Mar 24, 2025
View reviewed changes
@larissakl larissakl requested a review from curufinwe March 24, 2025 10:16
@curufinwe curufinwe changed the title Adjust AdvancedTreeSearch for CtcTreeBuilder support Modify AdvancedTreeSearch for CtcTreeBuilder support Mar 25, 2025
@curufinwe
Copy link
Contributor

@Stefanwuu Once you can confirm that these changes work for you too we can merge. I'll set my review to request changes for this purpose only for now.

curufinwe
curufinwe requested changes Mar 27, 2025
View reviewed changes
Copy link
Contributor

@curufinwe curufinwe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Waiting for confirmation that these changes are sufficient to run CTC decoding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@curufinwe curufinwe curufinwe requested changes

@SimBe195 SimBe195 SimBe195 approved these changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@larissakl @curufinwe @SimBe195